Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change README links to be doxygen-friendly #1927

Merged
merged 1 commit into from
Sep 25, 2024
Merged

Change README links to be doxygen-friendly #1927

merged 1 commit into from
Sep 25, 2024

Conversation

dstebila
Copy link
Member

Fixes #1897.

  • [No] Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
  • [No] Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider will also need to be ready for review and merge by the time this is merged.)

@dstebila dstebila self-assigned this Sep 17, 2024
@dstebila
Change README links to be doxygen-friendly
35f819d 
Signed-off-by: Douglas Stebila <dstebila@uwaterloo.ca>
jgoertzen-sb
jgoertzen-sb reviewed Sep 17, 2024
View reviewed changes
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@dstebila
Copy link
Member Author

It seems that Doxygen has strong opinions about upper/lowercase on anchors generated from Markdown, and these opinions differ from Github's choices. In my tests the links still scroll to the right anchors in the cross-referenced page, but someone else please confirm. It would be annoying to break cross-references in one program to make them work in another program, and vice versa.

@dstebila dstebila mentioned this pull request Sep 17, 2024
Closed
mouse07410
mouse07410 approved these changes Sep 22, 2024
View reviewed changes
Copy link

@mouse07410 mouse07410 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to say: while this PR is not perfect, it unbreaks building of docs. Thus, let us merge it, and add improvements later. Rather than argue about the best way, while gen_docs remains broken/unusable.

@dstebila dstebila added this to the 0.11.0 milestone Sep 23, 2024
@dstebila
Copy link
Member Author

Do we want to merge this now for inclusion in 0.11.0? I can't think of a risk that would merit a new release candidate, but want to check if anyone has another opinion.

@SWilson4
Copy link
Member

Do we want to merge this now for inclusion in 0.11.0? I can't think of a risk that would merit a new release candidate, but want to check if anyone has another opinion.

I can't imagine any risk in including this, and it would be nice to have working release docs on macOS.

@SWilson4
Copy link
Member

I will go ahead and hit merge, as there seems to be consensus (based on approvals/thumbs-ups) that (1) this is desirable for the release and (2) it's not necessary to cut a new RC.

@SWilson4 SWilson4 merged commit 18db4c6 into main Sep 25, 2024
81 checks passed
@dstebila
Copy link
Member Author

Thanks Spencer!

@dstebila dstebila deleted the ds-doxygen-links branch September 25, 2024 15:36
@praveksharma praveksharma mentioned this pull request Sep 27, 2024
Merged
@BrewTestBot BrewTestBot mentioned this pull request Sep 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgoertzen-sb jgoertzen-sb jgoertzen-sb left review comments

@mouse07410 mouse07410 mouse07410 approved these changes

@SWilson4 SWilson4 SWilson4 approved these changes

@praveksharma praveksharma praveksharma approved these changes

Assignees

@dstebila dstebila

Labels
None yet
Projects
None yet
Milestone
0.11.0
Development

Successfully merging this pull request may close these issues.

Current "main" fails to gen_docs
5 participants
@dstebila @SWilson4 @mouse07410 @praveksharma @jgoertzen-sb